Skip to content

type_density(): add echo.bw for reporting the smoothing bandwidth - #672

Merged
grantmcdermott merged 6 commits into
grantmcdermott:mainfrom
haomeng797-ship-it:echo-bw
Aug 1, 2026
Merged

type_density(): add echo.bw for reporting the smoothing bandwidth#672
grantmcdermott merged 6 commits into
grantmcdermott:mainfrom
haomeng797-ship-it:echo-bw

Conversation

@haomeng797-ship-it

Copy link
Copy Markdown
Contributor

Closes #287.

Adds an echo.bw argument to type_density() for reporting the smoothing bandwidth and the number of observations behind it, neither of which is visible from the curve.

tinyplot(~Sepal.Length | Species, iris, type = type_density(echo.bw = "sub"))

Destinations are "sub", "xlab", "cap", and "cat" (console, with "print" as a synonym), in any combination. TRUE is shorthand for "sub"; the default FALSE reports nothing.

Following your two calls in the issue: all bandwidths are reported, and the argument sits on type_density() rather than tpar(). The "cap" and "cat" destinations are the ones you suggested.

I took the formatting conventions from the code you linked, including the version that was removed in #284: sprintf("%.4g"), bracketed lists collapsed to three entries plus an ellipsis, and "Joint Bandwidth" when the bandwidth is shared. Output:

N = [50, 50, 50]   Joint Bandwidth = 0.1809                # joint.bw = "mean"
N = [50, 50, 50]   Bandwidth = [0.1229, 0.2124, 0.2073]    # joint.bw = "none"
N = 150   Bandwidth = 0.2736                               # single group
N = [100, 100, 100, ...]   Bandwidth = [0.3875, ...]       # more than three

That same code answered a question I had been about to ask: a destination the user has already labelled is left alone, matching if (is.null(dots[["xlab"]])) in your original. So sub = "..." supplied by the user survives echo.bw = "sub".

Two notes on the implementation:

  • The per-group lapply() is split into a lapply() that computes the densities and a Map() that reshapes them, so the bandwidths are still in hand when the label is built. To confirm this is a pure refactor, I rendered six configurations to SVG against main and diffed them: byte-identical with echo.bw off.
  • match_echo_bw() and format_echo_vec() are internal helpers, so NAMESPACE is unchanged.

The "Titles" section said the x-axis title omits the observation count and bandwidth, which echo.bw = "xlab" now contradicts, so it points at the new argument.

Tests: 16 non-snapshot assertions covering argument normalization, the two error paths, the display formatting, and the reported text for joint, individual, and single-group cases. I could not run the snapshot tests locally (macOS), so CI will need to confirm those.

One point where I departed from the first mock-up, and where a word from you would settle it. That mock-up calls type_density(echo.bw = 'sub'), so joint.bw is at its "mean" default, and the subtitle lists [0.1229, 0.2124, 0.2073]. Those are the three individual bandwidths, but under joint.bw = "mean" none of them is the bandwidth actually used: all three curves are drawn with their weighted mean, 0.1809. Reporting the individual values there seemed likely to suggest the groups were smoothed differently when they were not, so the rule I implemented is to report what was used: one value, named as joint, whenever the bandwidth is shared, and the full list only when the groups genuinely differ.

That reading also matches your answer in the issue, since "all of them" was in response to the case where bandwidths differ across groups. Both mock-ups reproduce exactly under it: joint.bw = "none" gives [0.1229, 0.2124, 0.2073], and joint.bw = "full" gives 0.2736. If you would rather the individual bandwidths always be shown, that is a one-line change.

@grantmcdermott

Copy link
Copy Markdown
Owner

This is great, thanks.

I updated/added some snapshot tests, since that's what we benchmark against in general.

The only remaining issue that I could see is that the "xlab" destination doesn't work. This could probably be fixed without too much effort. But I'm also okay with dropping this option if it's a pain.

pkgload::load_all("~/Documents/Projects/tinyplot/")
#> ℹ Loading tinyplot
plt(~ Sepal.Length, data = iris, type = type_density(echo.bw = "xlab"))

Created on 2026-07-31 with reprex v2.1.1

@grantmcdermott

Copy link
Copy Markdown
Owner

Oh, and feel free to add yourself as a contributor in the DESCRIPTION.

@haomeng797-ship-it

Copy link
Copy Markdown
Contributor Author

Thanks for adding the tests! I tracked down the xlab issue: the formula interface fills xlab with the variable name before type_data() runs, so the guard against overwriting user labels was mistaking the auto-derived label for a user-set one. Derived labels now carry a small tp_auto attribute on the value and the echo block checks that instead of NULL-ness. Verified that echo.bw = "xlab" now renders the bandwidth text and that an explicit user xlab is still left alone. Happy to rework the marker if you'd prefer a different mechanism.

And thanks for the DESCRIPTION invitation, added myself as ctb.

The formula interface fills xlab with the variable name before
type_data() runs, so the never-overwrite guard mistook the
auto-derived label for a user-set one and skipped it. Mark derived
labels with a tp_auto attribute on the value and have the echo block
check that instead of NULL-ness. User-supplied labels are still left
alone.
@grantmcdermott

Copy link
Copy Markdown
Owner

Derived labels now carry a small tp_auto attribute on the value and the echo block checks that instead of NULL-ness.

Hmmm. I'm probably being pedantic, but is it okay if we just drop the xlab support instead? I know that the automatic axes labeling for density (and histogram) types is a little bit tricksy. I'd prefer to avoid further custom catches and branching in the main code (c.f. #637).

I think supporting sub, cap, and cat should more than suffice for this functionality. Thanks for iterating with me.

Removes the tp_auto marker and the branching it needed in
sanitize_xylab() and tinyplot(), so the main code is back to what it
was. sub, cap, and cat remain.
@haomeng797-ship-it

Copy link
Copy Markdown
Contributor Author

Not pedantic at all, that's a fair call. The main code is back to what it was: I dropped the tp_auto marker and the branching it needed in sanitize_xylab() and tinyplot(). xlab is no longer a valid destination and errors with a message listing the three that are, and the docs and NEWS entry now only mention sub, cap, and cat.

@grantmcdermott
grantmcdermott merged commit 3316202 into grantmcdermott:main Aug 1, 2026
3 of 4 checks passed
@grantmcdermott

Copy link
Copy Markdown
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should we report bandwidth for type_density()?

2 participants